node: Remove minimum TxID length#4797
Open
johnsaigle wants to merge 2 commits into
Open
Conversation
johnsaigle
marked this pull request as ready for review
May 7, 2026 18:12
johnsaigle
requested review from
SEJeff,
bemic,
djb15,
evan-gray,
kcsongor,
mdulin2,
panoel and
pleasew8t
as code owners
May 7, 2026 18:12
mdulin2
previously approved these changes
Jun 15, 2026
mdulin2
reviewed
Jun 15, 2026
- include all fields in size check - add unit test to catch regressions in this area - move constant near others so divergences are easier to track - move AddressLength constant to where Address is defined
johnsaigle
force-pushed
the
fix-no-min-tx-id-len
branch
from
June 16, 2026 11:16
187a2e9 to
ad5cb34
Compare
Contributor
Author
|
@mdulin2 nice spot, I fixed the bug in the recent commits and also added a unit test for that particular case. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
#4428 added validation for Message Publications. This included a minimum Tx ID length check. However, this should not have been added as the Tx ID should be controlled by each watcher rather than enforced in the Message Publication itself.
In theory there is a current minimum length for TxIDs across all of the implementations but we should keep this field flexible.
Right now, Aptos is likely not marshalling correctly because its TxID is based on a uint64 value. In some cases it may be 'normalized' i.e. padded to 32 bytes to conform to other chains' IDs, but this shouldn't be necessary and enforced from a distance.